if lets say user has connected MCP servers with klavis, we want it to accessible#329
if lets say user has connected MCP servers with klavis, we want it to accessible#329shivammittal274 wants to merge 2 commits intomainfrom
Conversation
…we want it to accessible Task ID: XUfT6NJd
|
Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement. To sign the CLA, please add a comment to this PR with the following text: You only need to sign once. After signing, this check will pass automatically. Troubleshooting
You can retrigger this bot by commenting **recheck** in this Pull Request. Posted by the **CLA Assistant Lite bot**. |
Greptile OverviewGreptile SummaryThis PR consolidates Klavis MCP server integration by introducing a proxy layer that connects to Klavis Strata on startup, discovers upstream tools, and makes them available through the BrowserOS MCP endpoint. Previously, Klavis tools were only accessible when chat sessions explicitly requested them - now they're always available if the user has authenticated integrations. Key changes:
Issues found:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant App as Application (main.ts)
participant Proxy as KlavisMcpProxy
participant Klavis as KlavisClient
participant Strata as Klavis Strata
participant MCP as MCP Routes
participant Client as MCP Client
Note over App,Strata: Server Startup
App->>Proxy: new KlavisMcpProxy(klavisClient, browserosId)
App->>Proxy: connect()
Proxy->>Klavis: getUserIntegrations(browserosId)
Klavis-->>Proxy: [authenticated integrations]
Proxy->>Klavis: createStrata(browserosId, names)
Klavis-->>Proxy: strataServerUrl
Proxy->>Strata: client.connect(transport)
Strata-->>Proxy: connected
Proxy->>Strata: client.listTools()
Strata-->>Proxy: [upstream tools]
Note over Proxy: Start 5-min refresh interval
Note over MCP,Client: MCP Request Handling
Client->>MCP: MCP request
MCP->>MCP: createMcpServerWithTools()
Note over MCP: Register BrowserOS tools
Note over MCP: Register Klavis proxy tools
MCP->>Proxy: callTool(name, args)
Proxy->>Strata: client.callTool()
Strata-->>Proxy: result
Proxy-->>MCP: CallToolResult
MCP-->>Client: response
Note over Proxy,Strata: Integration Changes
Note over Proxy: User adds/removes integration
Proxy->>Proxy: refresh()
Proxy->>Klavis: getUserIntegrations()
Klavis-->>Proxy: [updated integrations]
alt Integration set changed
Proxy->>Proxy: disconnectClient()
Proxy->>Proxy: connect()
Proxy->>MCP: onToolsChanged()
MCP->>MCP: rebuild mcpServer
end
|
| }) | ||
|
|
||
| await this.disconnectClient() | ||
| await this.connect() |
There was a problem hiding this comment.
recursive connect() call during refresh() will restart the interval timer, causing multiple timers to run
Each call to connect() starts a new setInterval on line 78, but the old interval from the previous connection isn't cleared before reconnecting.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/lib/klavis-mcp-proxy.ts
Line: 146:146
Comment:
recursive `connect()` call during `refresh()` will restart the interval timer, causing multiple timers to run
Each call to `connect()` starts a new `setInterval` on line 78, but the old interval from the previous connection isn't cleared before reconnecting.
How can I resolve this? If you propose a fix, please make it concise.| deps.klavisMcpProxy.onToolsChanged = () => { | ||
| mcpServer = createMcpServerWithTools(deps) | ||
| logger.info('MCP server rebuilt with updated Klavis tools') | ||
| } |
There was a problem hiding this comment.
race condition: requests in-flight when tools change will use the old mcpServer instance
When onToolsChanged callback reassigns mcpServer, concurrent requests that already captured the old reference will continue using outdated tool registrations.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/api/routes/mcp.ts
Line: 203:206
Comment:
race condition: requests in-flight when tools change will use the old `mcpServer` instance
When `onToolsChanged` callback reassigns `mcpServer`, concurrent requests that already captured the old reference will continue using outdated tool registrations.
How can I resolve this? If you propose a fix, please make it concise.| await client.connect(transport) | ||
|
|
||
| const listResult = await client.listTools(undefined, { | ||
| signal: AbortSignal.timeout(TIMEOUTS.MCP_UPSTREAM_LIST_TOOLS), | ||
| }) |
There was a problem hiding this comment.
no timeout for client.connect() - could hang indefinitely
All other MCP operations use AbortSignal.timeout(), but the connection itself has no timeout protection.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/lib/klavis-mcp-proxy.ts
Line: 60:64
Comment:
no timeout for `client.connect()` - could hang indefinitely
All other MCP operations use `AbortSignal.timeout()`, but the connection itself has no timeout protection.
How can I resolve this? If you propose a fix, please make it concise.
Summary
if lets say user has connected MCP servers with klavis, we want it to accessible over browserOS MCP, so basically this will make only 1 mcp url then, if we can merge klavis to browseros mcp itself
Changes
Agent Metadata
Generated by coding-agent v3